-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
allow python3 tests without python installed #3301
Conversation
it returns both stdout and stderr closes ipythongh-3280
@@ -24,13 +25,15 @@ | |||
from IPython.testing import decorators as dec | |||
from IPython.testing import tools as tt | |||
|
|||
python = os.path.basename(sys.executable) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just use sys.executable
in the even more remote chance that python3
isn't in the PATH
either?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the find_cmd tes needs the basename and exe on path, so it doesn't really matter.
Everything else looks good to me, so merging. |
allow python3 tests without python installed
#----------------------------------------------------------------------------- | ||
# Tests | ||
#----------------------------------------------------------------------------- | ||
|
||
def test_find_cmd_python(): | ||
"""Make sure we find sys.exectable for python.""" | ||
nt.assert_equal(find_cmd('python'), sys.executable) | ||
nt.assert_equal(find_cmd(python), sys.executable) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this test fails on OS X - 'python'
is a special key handled by find_cmd short-circuited to sys.executable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but its still the string 'python'?
mac does not add extensions like windows so far I know
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the Python executable is not on the path, in a framework build (e.g. from Python.org), it's in the framework - something like /Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python
. You have really changed what this test means - it was a test for the short-circuit code:
does the string 'python' give back
sys.executable
?
But you changed it to something else:
does the string basename(sys.executable) give back
sys.executable
?
Which does not describe the behavior of the code this tests, so it fails in some cases. PR #3342 reconciles the code with the test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but its still the string 'python'?
To answer your question directly: no, it is Python
.
allow python3 tests without python installed
also closes gh-3280 via a documentation and test update